PUMAS round3 by cacraigucar · Pull Request #357 · ESCOMP/atmospheric_physics

cacraigucar · 2026-01-16T21:10:39Z

Originator(s): cacraigucar, nusbaume, billsacks, peverwhee, I-Kuan Hu, jimmielin

Description (include issue title and the keyword ['closes', 'fixes', 'resolves'] and issue number):

PUMAS code now has metadata and goes through the CAM-SIMA compilation step. [Note PUMAS diagnostics will come in a later stage] - This step has been named "round3".
This commit contains contributions from participants at the PUMAS hackathon
Partially addresses CCPP-ize PUMAS #118

List all namelist files that were added or changed:
A schemes/pumas/pumas_pre_main_namelist.xml
- pumas CAM-SIMA namelist generator

List all files eliminated and why: N/A

List all files added and what they do:
A schemes/pumas/micro_pumas_ccpp_dimensions_post.F90
A schemes/pumas/micro_pumas_ccpp_dimensions_post.meta
A schemes/pumas/micro_pumas_ccpp_dimensions_pre.F90
A schemes/pumas/micro_pumas_ccpp_dimensions_pre.meta
- code which subsets and converts to pumas_r8 and back

A schemes/pumas/pumas_pre_main.F90
A schemes/pumas/pumas_pre_main.meta
A schemes/pumas/pumas_post_main.F90
A schemes/pumas/pumas_post_main.meta
- placeholder for interstitial schemes

A schemes/sima_diagnostics/pumas_diagnostics.F90
A schemes/sima_diagnostics/pumas_diagnostics_calling.F90
A schemes/sima_diagnostics/pumas_diagnostics.F90-keep_for_run
- These are being included even though they are only partially complete - NO REVIEW IS BEING DONE NOW

A test/test_suites/suite_pumas.xml
- test suite for PUMAS

A schemes/microp_aero/ccpp/hetfrz_classnuc_stub.F90
A schemes/microp_aero/ccpp/hetfrz_classnuc_stub.meta
A schemes/microp_aero/ccpp/hetfrz_classnuc_stub_namelist.xml
- stub for hetfrz_classnuc to provide the namelist option do_hetfrz_classnuc used by PUMAS core (micro_pumas_ccpp)

A schemes/pumas/micro_pumas_ccpp_optics_limiter.F90
A schemes/pumas/micro_pumas_ccpp_optics_limiter.meta
- optics property limiter for strat_cldfrc<cldfrc_min copied from micro_pumas_cam

A schemes/utilities/clamp_number_concentrations.F90
A schemes/utilities/clamp_number_concentrations.meta
- clamp number concentrations to 1e-12, 1e10 range the same as CAM physics types

A test/test_schemes/clubb_stub.F90
A test/test_schemes/clubb_stub.meta

List all existing files that have been modified, and describe the changes:
(Helpful git command: git diff --name-status development...<your_branch_name>)
M .gitmodules
M doc/NamesNotInDictionary.txt
M schemes/pumas/pumas

M schemes/rasch_kristjansson/rk_stratiform.F90
M schemes/rasch_kristjansson/rk_stratiform.meta
M schemes/rrtmgp/rrtmgp_lw_cloud_optics.meta
M schemes/rrtmgp/rrtmgp_sw_cloud_optics.meta
M schemes/sima_diagnostics/rk_stratiform_diagnostics.F90
M schemes/sima_diagnostics/rk_stratiform_diagnostics.meta
M test/test_schemes/initialize_constituents.F90
- updated standard names

List all automated tests that failed, as well as an explanation for why they weren't fixed:

Is this an answer-changing PR? If so, is it a new physics package, algorithm change, tuning change, etc?
- new capability in CAM-SIMA

If yes to the above question, describe how this code was validated with the new/modified features:

…ns_pre' init phase.

atmos_phys0_20_000: Merge development to main (ESCOMP#345)

…mment to top for reviewers

…rtially implemented run mods)

Merge development into main (ESCOMP#361)

Fixes for PUMAS bfb with QPC7 and FHISTC_LTso CAM snapshot

jimmielin · 2026-06-23T00:23:56Z

Please note that since compute_cloud_fraction_two_moment has been merged, rhmini in micro_pumas_ccpp can now use its value via the namelist and the hardcoded value in registry.xml for CAM-SIMA can also be removed. Please see nusbaume/PUMAS#1

…o producers

…s_cam

… pumas_post_main Mirrors CAM micro_pumas_cam post-call recomputation, which discards the per-substep values PUMAS returns. Match dei standard name with RRTMGP and the CAM-SIMA registry (singular crystal). Run pumas_post_main before apply_heating_rate (air density uses the pre-heating temperature) and add the low stratiform cloud fraction optics limiter to the suite.

Mirrors CAM micro_pumas_cam, including the separate meters-to-microns conversion step. Match snow diameter standard name with RRTMGP and the CAM-SIMA registry (snow_crystal).

Updates for bit-for-bit fields that interact with RRTMGP

atmos_phys0_26_000: Move ndrop_bam, nucleate_ice, and other microp_aero interstitials to atmos_phys (ESCOMP#411)

nusbaume · 2026-07-22T16:44:16Z

Automated code review (PR #357 — PUMAS round3)

This is an AI-assisted review (claude-opus:4.8) against the rules in doc/Code-review.md, doc/Code-style.md, and AGENTS.md. Per the PR description, pumas_diagnostics.F90 and pumas_diagnostics_calling.F90 are explicitly not being reviewed at this stage, so I've excluded them (I did note a couple of things about them at the bottom for later). Auto-generated doc/NamesNotInDictionary.txt and the submodule pointer/.gitmodules bumps were not reviewed.

High priority

Missing implicit none — schemes/pumas/micro_pumas_ccpp_dimensions_post.F90 (Code-style MUST). The module preamble goes straight from the use statements to public :: with no implicit none. Every module must have implicit none in the preamble.
errmsg must be character(len=*), not character(len=512) (Code-review rule 11). The rule explicitly covers CCPP-framework variables such as errmsg. This affects:
- schemes/pumas/micro_pumas_ccpp_dimensions_pre.F90 (both subroutines)
- schemes/pumas/micro_pumas_ccpp_dimensions_post.F90
- schemes/pumas/micro_pumas_ccpp_optics_limiter.F90
- schemes/pumas/pumas_pre_main.F90 (both subroutines)
- schemes/pumas/pumas_post_main.F90
- schemes/utilities/clamp_number_concentrations.F90 (both subroutines)
- test/test_schemes/clubb_stub.F90
For reference, schemes/microp_aero/ccpp/hetfrz_classnuc_stub.F90 (added in this same PR) already does this correctly with character(len=*).
Module-level save — schemes/utilities/clamp_number_concentrations.F90:13 (Code-review rule 1). Module variables are implicitly saved, so the explicit module-level save is redundant and should be removed.

Medium priority

endif → end if (Code-review rule 4) — schemes/pumas/pumas_pre_main.F90:46 (the if (do_clubb_sgs) block).
Commented-out code (Code-review rule 13) — schemes/utilities/clamp_number_concentrations.F90. The _run subroutine ends with a large block of commented-out alternative implementation (the q_projected = ... formulation, ~30 lines). The explanatory rationale is valuable, but per rule 13 the dead code itself should be removed (keep the explanation as prose if you want the reasoning preserved). Note this also leaves the declared local q_projected unused in the active code.
Namelist XML missing the standard comment block (Code-review rule 6). Neither new namelist file has the standard  block that follows <entry_id_pg version="2.0"> in the other namelist XMLs (e.g. dadadj_namelist.xml):
- schemes/microp_aero/ccpp/hetfrz_classnuc_stub_namelist.xml
- schemes/pumas/pumas_pre_main_namelist.xml
Missing default private + explicit public (Code-style: modules are default private with explicit public interfaces).
- schemes/pumas/micro_pumas_ccpp_dimensions_pre.F90 (no private, no public)
- schemes/pumas/micro_pumas_ccpp_dimensions_post.F90 (has public but no private)
- schemes/pumas/pumas_pre_main.F90 (no private, no public)

Low priority / for human review

Comment grammar/typo (Code-review rule 9) — schemes/pumas/pumas_pre_main.F90 header. "The hope that there will eventually be one set of portable interstitials..." is a sentence fragment (missing "is" — "The hope is that..."), and "intersititals" is misspelled (→ "interstitials").
schemes/sima_diagnostics/pumas_diagnostics.F90-keep_for_run (2150 lines added). This file has a non-.F90 extension and appears to be a leftover working/backup copy committed by accident (GitHub can't even diff it). It falls under the diagnostics work you've flagged as out-of-review, but a -keep_for_run file almost certainly should not be committed to the repo — please confirm/remove.
(Optional, Code-review rule 3 — pure.) The pure data-reshaping converters do no I/O and have no side effects, so they are candidates for the pure keyword: micro_pumas_ccpp_dimensions_pre_*, micro_pumas_ccpp_dimensions_post_run, micro_pumas_ccpp_optics_limiter_run, and clamp_number_concentrations_run. (pumas_post_main_run calls external size_dist_param_* routines, so it likely can't be pure.) Flagging as optional since it isn't universal practice for CCPP entry points here.
(Minor, Code-style modern syntax.) schemes/utilities/clamp_number_concentrations.F90:~112 uses the old array-constructor (/ ... /); prefer [ ... ].

Notes (not blocking)

The effective_radius_of_stratiform_cloud_{liquid_water,ice}_particle → ..._droplet/..._crystal and effective_diameter_..._particle_for_radiation → ..._crystal_for_radiation renames are applied consistently across rk_stratiform.{F90,meta}, rrtmgp_{lw,sw}_cloud_optics.meta, and rk_stratiform_diagnostics.{F90,meta}, with units unchanged (um). No rule-14 (same standard_name, differing units) conflicts were found in the modified .meta files.
The new mass_number_concentration_*_{droplets,drops,crystals,particles}_in_moist_air_and_condensed_water names in initialize_constituents.F90 are consistent with their use in clamp_number_concentrations.F90. As a reminder (AGENTS.md), any newly coined standard names should be explicitly called out in the PR description for SE review.
For later: pumas_diagnostics.F90 and pumas_diagnostics_calling.F90 also carry module-level save statements (rule 1) — worth addressing when those files come up for review.

nusbaume

Thanks for getting all of the required interstitials set up @cacraigucar and @jimmielin! I have some comments and change requests, but I believe many should either be easy to fix, or can just be pushed off onto future work issues (including the standard name requests). Of course if you have any questions or concerns just let me know!

nusbaume · 2026-07-22T23:36:15Z

+
+    !CCPP error handling:
+    character(len=*), intent(out) :: errmsg
+    integer,            intent(out) :: errcode


Fix indentation here to better align with the code above?

Suggested change

integer, intent(out) :: errcode

integer, intent(out) :: errcode

nusbaume · 2026-07-22T23:57:24Z

+    !Host model dimensions/parameters:
+    integer,         intent(in) :: ncol
+    integer,         intent(in) :: nlev
+    integer,         intent(in) :: nlevp1
+    integer,         intent(in) :: trop_cloud_top_lev  !Index of the top model level for which
+                                                       !cloud physics is applied (1 to nlev)
+
+    !PUMAS dimensions/parameters:
+    integer,         intent(in) :: micro_ncol          !Number of horizontal microphysics columns (count)
+    integer,         intent(in) :: micro_nlev          !Number of microphysics vertical layers (count)
+    integer,         intent(in) :: micro_nlevp1        !Number of microphysics vertical interfaces (count)
+    integer,         intent(in) :: micro_dust_nbins    !Number of dust bins


It looks like most of these variables aren't used in this subroutine, so I think this can be cleaned up to be the following:

Suggested change

!Host model dimensions/parameters:

integer, intent(in) :: ncol

integer, intent(in) :: nlev

integer, intent(in) :: nlevp1

integer, intent(in) :: trop_cloud_top_lev !Index of the top model level for which

!cloud physics is applied (1 to nlev)

!PUMAS dimensions/parameters:

integer, intent(in) :: micro_ncol !Number of horizontal microphysics columns (count)

integer, intent(in) :: micro_nlev !Number of microphysics vertical layers (count)

integer, intent(in) :: micro_nlevp1 !Number of microphysics vertical interfaces (count)

integer, intent(in) :: micro_dust_nbins !Number of dust bins

!Host model dimensions/parameters:

integer, intent(in) :: ncol

integer, intent(in) :: trop_cloud_top_lev !Index of the top model level for which

!cloud physics is applied (1 to nlev)

nusbaume · 2026-07-23T02:16:09Z

+  use pumas_kinds,       only: pumas_r8=>kind_r8
+  use ccpp_kinds,        only: kind_phys


This should have been caught by the AI, but can we move these use statements down to the relevant subroutine level?

nusbaume · 2026-07-23T02:55:32Z

+  advected = true
+[pumas_cldliq]
+  standard_name = pumas_cloud_liquid_water_mixing_ratio_wrt_moist_air_and_condensed_water
+  long_name = microphysics cloud liquid wrt moist air and condensed water of new state


I might add "mixing ratio" to the long name here:

Suggested change

long_name = microphysics cloud liquid wrt moist air and condensed water of new state

long_name = microphysics cloud liquid mixing ratio wrt moist air and condensed water of new state

nusbaume · 2026-07-23T03:12:30Z

+[pumas_frzimm]
+  standard_name = tendency_of_pumas_cloud_liquid_droplet_number_concentration_due_to_immersion_freezing
+  long_name = microphysics tendency of cloud liquid droplet number concentration due to immersion freezing
+  units = cm-3


I am not sure why I didn't catch this earlier, but essentially by definition a variable that is a tendency_of_X must have a s-1 term in the units, which it looks like the three frz variables listed here don't have.

I wouldn't worry about it for this PR, but it might be good to create an issue pointing out this discrepency, as it could potentially indicate either a science bug or an incorrect description of what the variable actually is.

Just a note to feel free to assign this issue to me as I will deal with the units when CCPPizing hetfrz_classnuc eventually, beyond the MAM work.

nusbaume · 2026-07-24T18:17:31Z

@@ -0,0 +1,1206 @@
+[ccpp-table-properties]
+  name = micro_pumas_ccpp_dimensions_post
+  type = scheme


Don't we need a dependencies and kind_spec attribute here?

Suggested change

type = scheme

type = scheme

dependencies = pumas/pumas_kinds.F90

kind_spec = pumas_kinds:pumas_r8=>kind_r8

nusbaume · 2026-07-24T18:21:55Z

+  kind = pumas_r8
+  intent = in
+[numice_tend]
+  standard_name = tendency_of_mass_number_concentration_of_ice_wrt_moist_air_and_condensed_water


I believe this should say cloud_ice_water_crystals:

Suggested change

standard_name = tendency_of_mass_number_concentration_of_ice_wrt_moist_air_and_condensed_water

standard_name = tendency_of_mass_number_concentration_of_cloud_ice_water_crystals_wrt_moist_air_and_condensed_water

nusbaume · 2026-07-24T19:11:13Z

+    rho(:,:)     = real(pmid(:ncol,:), pumas_r8) / &
+                (real(rair, pumas_r8) * real(temp(:ncol,:), pumas_r8))
+    icwmrst(:,:) = min(real(cldliq(:ncol,:), pumas_r8) / &
+                max(mincld, real(strat_cldfrc(:ncol,:), pumas_r8)), 0.005_pumas_r8)
+    ncic(:,:)   =    real(numliq(:ncol,:), pumas_r8) / &
+                max(mincld, real(strat_cldfrc(:ncol,:), pumas_r8))
+
+    mu(:,:)      = 0._pumas_r8
+    lambdac(:,:) = 0._pumas_r8
+    do k = trop_cloud_top_lev, nlev
+      call size_dist_param_liq(mg_liq_props, icwmrst(:,k), ncic(:,k), rho(:,k), &
+                               mu(:,k), lambdac(:,k), ncol)
+    end do


There might be a science bug here, in that the rho is calculated using the dry air gas constant and regular temperature (but the full pressure), and is then being used to convert the value of various mass number concentration variables (like ncic), which is with respect to the mass of moist air and condensed water. I obviously wouldn't worry about fixing it here, but might be worth opening an issue so that we can double-check it in the future?

Also pinging @PeterHjortLauritzen in case I am totally wrong here.

Just a note that I think this bug belongs to ESCOMP/PUMAS.

The post-interstitial rho is consistent with the one inside micro_pumas_v1.F90

rho(i,k) = p(i,k)/(r*t(i,k))

p here is also pmid, r is rair

The rho cancels out in des and degrau, I think, so it's just pgam and lamc.

If a fix is made it should be in PUMAS then the downstream interstitials are adjusted to remain consistent. This would introduce answer changes everywhere, though.

nusbaume · 2026-07-24T19:30:24Z

+  type = scheme
+  dependencies = pumas/micro_pumas_utils.F90


Do we need the kind-related dependencies here as well?

Suggested change

type = scheme

dependencies = pumas/micro_pumas_utils.F90

type = scheme

dependencies = pumas/micro_pumas_utils.F90

dependencies = pumas/pumas_kinds.F90

kind_spec = pumas_kinds:pumas_r8=>kind_r8

nusbaume · 2026-07-24T19:32:57Z

+  intent = in
+  advected = true
+[numice]
+  standard_name = mass_number_concentration_of_ice_wrt_moist_air_and_condensed_water


I believe this should be cloud_ice_water_crystals here:

Suggested change

standard_name = mass_number_concentration_of_ice_wrt_moist_air_and_condensed_water

standard_name = mass_number_concentration_of_cloud_ice_water_crystals_wrt_moist_air_and_condensed_water

nusbaume and others added 25 commits October 2, 2025 13:26

Add PUMAS directory with stub interstitial file.

a9328ae

Add wv_sat_methods.F90, which is needed by PUMAS.

e7205c3

Add PUMAS submodule.

8b56c47

Add init phase and metadata file to 'pre_main' scheme.

301f43e

Add pumas_pre_main_namelist.xml based on micro_pumas_ccpp.meta

ece068b

add pre and post dimension mapping schemes

659327a

add constituent metadata; fix stdname

b188b67

Moved from MP/PUMAS to pumas directory

6b3cc4b

reorder pumas directory

50be090

Add hackathon work from Kate and Cheryl

3ec547c

Add pumas test_suite

ffbac28

remove unused directory

07b71dc

Update NamesNotInDictionary.txt

335b649

Remove unused 'src/physics/pumas' directory.

fcc8453

Fix unit metadata bugs in PUMAS and PUMAS namelist definition file.

88c2e98

Update logical variable standard names, and create new 'ccpp_dimensio…

80ae20a

…ns_pre' init phase.

fixes to get further in a CAM-SIMA build

1bc8363

Updates to support accum_enhan

90fed13

Next round of changes which compiled in CAM

171fa8e

Remove extra copy - do subsetting and pumas_r8 conversion in same step

7443df3

Fixes for CAM-SIMA prebuild processing

01d55b0

Merge tag 'atmos_phys0_20_000' into pumas_round3

8b359bf

atmos_phys0_20_000: Merge development to main (ESCOMP#345)

Fix compiler errors

485736b

update pumas external

2b8b259

update pumas external

55fb203

cacraigucar marked this pull request as draft January 16, 2026 21:11

cacraigucar added 4 commits January 16, 2026 16:57

Verified pumas_diagnostics routines are not currently used - Added co…

ff987b8

…mment to top for reviewers

Mods for diagnostics init (and placeholder file which contains the pa…

9050827

…rtially implemented run mods)

Merge tag 'atmos_phys0_21_000' into pumas_round3

263076d

Merge development into main (ESCOMP#361)

Merge commit '87e76a' into pumas_round3

36a7324

cacraigucar added 3 commits June 18, 2026 12:36

Merge pull request #1 from jimmielin/pumas_round3

2b7ecfa

Fixes for PUMAS bfb with QPC7 and FHISTC_LTso CAM snapshot

Merge remote-tracking branch 'upstream/main' into pumas_round3

94c5548

Update standard names

22a3457

cacraigucar requested a review from nusbaume June 18, 2026 21:38

cacraigucar marked this pull request as ready for review June 18, 2026 21:38

jimmielin and others added 20 commits July 14, 2026 16:39

Match standard names with RRTMGP names in atmos_phys

4b9ea65

Match naai and dust size bin dimension standard names with microp_aer…

31d342b

…o producers

Update microp_aero standard names

3aa3f24

Update Standard names

6e18593

Merge remote-tracking branch 'haipeng/pumas_round3' into pumas_round3

2a31c57

Merge remote-tracking branch 'upstream/main' into pumas_round3

835c233

Add cloud schemes to PUMAS SDF

2226c40

Update .gitmodules

26beeb8

Put back files from Haipeng

4656a6b

Port low stratiform cloud fraction optics limiter from CAM micro_puma…

787c32c

…s_cam

Update PUMAS external to 59cebff (jesse/ccpp_fixes tip, matches fxtag)

e5494a7

Recompute des (snow effective diameter) for radiation in pumas_post_main

70cc4db

Mirrors CAM micro_pumas_cam, including the separate meters-to-microns conversion step. Match snow diameter standard name with RRTMGP and the CAM-SIMA registry (snow_crystal).

More robust fixes for the clamp and initialize; dimensions_pre unit fix

9b6e450

Dimension and various bfb fixes

8aa7e54

Add back qneg to suite

2ea30ba

Change to avoid capgen bug.

e56dc31

Merge pull request #3 from jimmielin/pumas_round3

e2f5ec2

Updates for bit-for-bit fields that interact with RRTMGP

Update to official PUMAS tag

25faf11

Merge tag 'atmos_phys0_26_000' into pumas_round3

2797786

atmos_phys0_26_000: Move ndrop_bam, nucleate_ice, and other microp_aero interstitials to atmos_phys (ESCOMP#411)

cacraigucar added 2 commits July 22, 2026 15:14

Address AI review comments

4bc5b7f

Add pumas external

b8fec30

nusbaume requested changes Jul 24, 2026

View reviewed changes

	integer, intent(out) :: errcode
	integer, intent(out) :: errcode

		use pumas_kinds, only: pumas_r8=>kind_r8
		use ccpp_kinds, only: kind_phys

	long_name = microphysics cloud liquid wrt moist air and condensed water of new state
	long_name = microphysics cloud liquid mixing ratio wrt moist air and condensed water of new state

	standard_name = tendency_of_mass_number_concentration_of_ice_wrt_moist_air_and_condensed_water
	standard_name = tendency_of_mass_number_concentration_of_cloud_ice_water_crystals_wrt_moist_air_and_condensed_water

	standard_name = mass_number_concentration_of_ice_wrt_moist_air_and_condensed_water
	standard_name = mass_number_concentration_of_cloud_ice_water_crystals_wrt_moist_air_and_condensed_water

Uh oh!

Conversation

cacraigucar commented Jan 16, 2026 • edited by jimmielin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jimmielin commented Jun 23, 2026

Uh oh!

nusbaume commented Jul 22, 2026

Automated code review (PR #357 — PUMAS round3)

High priority

Medium priority

Low priority / for human review

Notes (not blocking)

Uh oh!

nusbaume left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cacraigucar commented Jan 16, 2026 •

edited by jimmielin

Loading